[Preprocessor] Do not expand macros if the input is already preprocessed #137665
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Juan Manuel Martinez Caamaño (jmmartinez) ChangesThis is a draft while I'm trying to figure out what's left to do. This has issues with the test I'm not familiar with modules and I'm not sure if it's this patch, or the test that has issues. Other failures are fixed with #137623 Full diff: https://github.com/llvm/llvm-project/pull/137665.diff 3 Files Affected:
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index f2dfd3a349b8b..63774e48a468b 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1831,6 +1831,11 @@ class Preprocessor {
MacroExpansionInDirectivesOverride = true;
}
+ void SetDisableMacroExpansion() {
+ DisableMacroExpansion = true;
+ MacroExpansionInDirectivesOverride = false;
+ }
+
/// Peeks ahead N tokens and returns that token without consuming any
/// tokens.
///
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 1f297f228fc1b..6693cfb469f82 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1556,6 +1556,13 @@ void clang::InitializePreprocessor(Preprocessor &PP,
const PCHContainerReader &PCHContainerRdr,
const FrontendOptions &FEOpts,
const CodeGenOptions &CodeGenOpts) {
+
+ if (all_of(FEOpts.Inputs,
+ [](const FrontendInputFile &FI) { return FI.isPreprocessed(); })) {
+ PP.SetDisableMacroExpansion();
+ return;
+ }
+
const LangOptions &LangOpts = PP.getLangOpts();
std::string PredefineBuffer;
PredefineBuffer.reserve(4080);
diff --git a/clang/test/Preprocessor/preprocess-cpp-output.c b/clang/test/Preprocessor/preprocess-cpp-output.c
new file mode 100644
index 0000000000000..2c180601e30ac
--- /dev/null
+++ b/clang/test/Preprocessor/preprocess-cpp-output.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -E -x c %s | FileCheck %s --check-prefixes=EXPANDED
+// RUN: %clang_cc1 -E -x cpp-output %s | FileCheck %s --check-prefixes=NOT-EXPANDED
+
+// EXPANDED: void __attribute__((__attribute__((always_inline)))) foo()
+// NOT-EXPANDED: void __attribute__((always_inline)) foo()
+
+#define always_inline __attribute__((always_inline))
+void __attribute__((always_inline)) foo() {
+ return 4;
+}
|
|
module maps don't get preprocessed, so I don't think it matters what that is set to. For normal module.modulemap files |
Thanks for the pointer. @Bigcheese who should we solicit an approving review from? I'm not familiar enough with the implications of the change to feel confident approving |
Currently a single test breaks: I guess then that this is an issue with the test. I'll try to propose a fix to the test then. |
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
4ed49c6 to
09c37ac
Compare
|
I fixed the |
|
This seems reasonable. Can you update the description to explain the motivation for the change, though? |
I've updated the description with an explanation of the situation that triggered the issue for us. But in the wider scheme, I think that no preprocessing the preprocessor's output is the actual desired behavior. Thanks! |
|
Ping ! |
Sorry, I missed that you replied last week |
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
9099d6f to
1c9b89b
Compare
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
1c9b89b to
89ace07
Compare
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
89ace07 to
335c604
Compare
|
Hello ! I’m still exploring possible alternatives to this PR but I haven't found anything satisfying yet. If you have some suggestions don't hesitate. Thanks ! |
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
335c604 to
2298cdc
Compare
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
ba24dc8 to
fbe9e06
Compare
|
@cor3ntin @AaronBallman any thoughts on this PR? Aiming to land soon if there are no other objections |
cor3ntin
left a comment
cor3ntin
left a comment
There was a problem hiding this comment.
LGTM. Please wait ~20h in case @AaronBallman @erichkeane have further comments
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
A nit and a question for the modules maintainers, but otherwise I think this looks good to me. Please don't land until @Bigcheese or @ChuanqiXu9 have had a chance to weigh in on the modules test question.
jmmartinez
force-pushed
the
draft/do-not-preprocess-cpp-output
branch
from
352f3f1 to
00c9355
Compare
6 participants
Preprocessing the preprocessor output again interacts poorly with some flag combinations when we perform a separate preprocessing stage. In our case,
-no-integrated-cpp -dDtriggered this issue; but I guess that other flags could also trigger problems (-save-tempsinstead of-no-integrated-cpp).Full context (which is quite weird I'll admit):
-no-integrated-cppfor the driver to generate a separate preprocessing command (clang -E) before the rest of the compilation.opencl-c-base.h). The semantic analysis queries the preprocessor to check if these are defined or not, for example, when we checks if a builtin is available when using-fdeclare-opencl-builtins.#definedirectives, on the preprocessor's output, we use-dD. However, other#definedirectives are also maintained besides OpenCL ones; which triggers the issue shown in this PR.A better fix for our particular case could have been to move the language features implemented as macros into some sort of a flag to be used together with
-fdeclare-opencl-builtins.But I also thought that not preprocessing preprocessor outputs seemed like something desirable. I hope to work on this on a follow up.